Skip to content

feat(admin): localize the operator app with react-i18next (3/4) - #1383

Open
marcelo-maciel wants to merge 15 commits into
fullstackhero:mainfrom
marcelo-maciel:feat/i18n-admin
Open

marcelo-maciel wants to merge 15 commits into
fullstackhero:mainfrom
marcelo-maciel:feat/i18n-admin

Conversation

@marcelo-maciel

@marcelo-maciel marcelo-maciel commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Carries two infrastructure fixes that are not this PR's topic. Without them CI cannot even reach this PR's code.

  • Dependency bump: Microsoft.SourceLink.GitHub to 10.0.401 and Testcontainers to 4.14.0, clearing NU1902/NU1903 so restore succeeds. Those are the versions #1375 (SourceLink) and #1369 (Testcontainers) carry: this hunk is the union of the two, plus one comment per pin naming the advisory it answers.
  • MinIO (c6a72df1): minio/minio is gone from Docker Hub, so every Testcontainers-backed integration test dies on the image pull. Pulls from quay.io on a pinned tag instead. Same fix as #1388.

The MinIO hunk is byte-identical to #1388. The dependency hunk is not byte-identical to #1375 or #1369, which each carry half of it without the comments, but it is identical across all twelve PRs in this series: src/Directory.Packages.props resolves to the same blob (854deb95) at every head. Either way they merge in any order, and these copies can be dropped once the PRs that own them land.

Reopened from #1362. That PR was closed automatically on 2026-09-14, when the head fork
was deleted. It reopened at a10d3976, and review has added commits on top since then (the
commit list above is the current one). The earlier review history stays on #1362.


clients/admin slice of the i18n work, split out of #1344. 111 files, of which 30 are JSON catalogs. Mostly mechanical, as you predicted.

Depends on #1381 for server-side persistence. The switcher works on its own — i18next persists the choice in localStorage — but the locale field this slice sends on PUT /identity/profile is only accepted once #1381 adds it to FshUser, UserDto and UpdateUserCommand. Until then the server round-trip is a no-op, so carrying a language across devices and minting the locale claim into the refreshed token both need #1381 merged first. Nothing here blocks the other slices.

Slice Files PR
Framework 56 #1381 — the hard review
Module catalogs and wiring 229 #1382 — depends on the framework slice
clients/admin 111 this one
clients/dashboard 134 #1384 — independent

What is in here

  • react-i18next wiring in src/i18n.ts. The chosen language is persisted to the user profile and sent to the API as Accept-Language through apiFetch; variants are canonicalised onto a supported tag before the call, so the API never sees a bare pt.
  • en-US and pt-BR catalogs, split per feature namespace, held at strict key and placeholder parity in both directions by tests/i18n/parity.spec.ts — a missing or mis-arged translation fails the build instead of shipping English.
  • Language switcher in the topbar, and html[lang] follows the active language through a languageChanged listener. The app previously shipped a static lang="en" that nothing updated, so a Portuguese UI announced itself as English to screen readers and browser translation.
  • Formatting stays in the presentation layer (src/lib/format.ts). That is the other half of the framework slice's UI-culture-only decision: the API pins CultureInfo.CurrentCulture to invariant and the app formats numbers, dates and currency itself.
  • Impersonation handoff carries locale in the URL. StartImpersonation strips the target's locale claim on purpose, and the two apps normally sit on different origins, so there was no other way to convey it and the API fell through to the dashboard's own browser detection. Harmless on its own: on main nothing reads the parameter until the dashboard slice lands, and this PR's handoff-locale.spec.ts passes with only this half present.

Testing

Everything below is this slice on its own, at main plus these 111 files — not a share of the unsplit branch's totals.

  • npm ci, npm run build (tsc -b + vite build), npx tsc -b tsconfig.tests.json and npm run lint: all exit 0.
  • Playwright, full suite, one worker: 153 passed, 0 failed, 0 flaky, 0 skipped. That is 136 at the time of the split (135 plus the single-flight test below, i.e. the unsplit branch's count plus one, so nothing was lost in the cut) plus the review follow-ups below. That includes tests/impersonation/handoff-locale.spec.ts, which passes with only this half of the handoff present.

Token refresh is single-flight

The server rotates the refresh token on every successful call, so two refreshes overlapping means the second one presents a token the server has already spent: it gets a 401. The switcher made that reachable, since it refreshes right after persisting the language. It used to end the session outright; see the first follow-up below for the other half of that fix.

The guard lives inside refreshAccessToken() in clients/admin/src/lib/api-client.ts rather than at any one call site, because three paths reach it — the 401 retry in apiFetch, session bootstrap, and the switcher — and any two overlapping is enough. tests/i18n/switcher.spec.ts drives two quick language switches and asserts one refresh request and a session that survives.

src/Directory.Packages.props

One backend file in a React PR, which needs explaining. template-smoke.yml triggers on paths: clients/** and runs dotnet build on the scaffolded solution, and .template.config/template.json does not exclude src/Tests/** — so a front-end-only PR still restores the full package graph and hits NU1903 / GHSA-q939-rpr3-3284 on SSH.NET 2025.1.0, pulled transitively by Testcontainers. That advisory fails restore on main too, re-verified today at 3f2959e6.

What that file carries is the Testcontainers and SourceLink bumps, byte-identical to #1378, so both stay mergeable in either order and this copy can be dropped once #1378 lands. The explicit SSH.NET pin it used to carry is gone: Testcontainers 4.14.0 already depends on the patched version. See the note at the end.

Notes

  • The topbar's language hydration stops once the user chooses a language in-session, guarding against a stale profile save echoing the old locale back. That guard has no regression test: reproducing it needs an in-mount profile refetch driven through the Settings form, and the click races the language-change re-render (element detached from the DOM). Three distinct approaches, then stopped rather than paper over it with retries or a longer timeout. The underlying lost update on PUT /identity/profile is tracked in #1359, where the ponytail: comments in the topbar point.
  • SignalR does not carry the app locale: the hub client builds its own requests instead of going through apiFetch, so Accept-Language on the negotiate is the browser's. Applies to every session, not just impersonation. (An earlier version of this line claimed handoff-locale.spec.ts named the channel explicitly. It does not — the spec only asserts the handoff URL parameters. Corrected rather than left standing.)

Docs (Golden Rule #10)

fullstackhero/docs#238, kept as a single PR covering all four slices — internationalization.mdx is one page whose sections map across the split. The Frontend (admin and dashboard) section is this slice and the dashboard one: catalogs, language detection and normalization, Accept-Language, the switcher and locale-aware formatting. That PR should merge after the last of the four, so the page never describes code that is not on main yet.

Review follow-ups

An independent review of this slice found one P1 and a few smaller things. All are fixed here:

  • Choosing a language could end the session. The single-flight stopped two concurrent refreshes
    from racing, but one failed refresh was still enough on its own: refreshAccessToken() called
    tokenStore.clear() on any non-ok response, so a refresh token that had been revoked, rotated in
    another tab or dropped by a reseed signed the operator out for picking a language. Clearing the
    session now belongs to the callers that know the request needed auth (the 401 retry in apiFetch
    and the boot probe in AuthProvider); the switcher reports the failure instead of swallowing it.
    tests/i18n/switcher.spec.ts covers the single-switch case alongside the double-switch one.
  • A failed save was invisible. The language mutation had an onSuccess and no onError, so a
    rejected PUT /identity/profile left the UI switched with nothing to tell the user the choice was
    not stored. It now raises a toast.
  • Catalog parity now covers interpolation, not just keys. A translation that drops or renames
    {{var}} renders the placeholder as literal text and no key is missing, so the old gate passed.

Second round

A second independent review found a P1 this slice introduced:

  • Approving a top-up printed status.invoiced in the badge. The label key is built from the
    value the API sends, and the catalog had neither status.invoiced nor status.cancelled — while
    it did have status.approved, a status the backend never emits. On main that badge printed the
    raw enum name, so this slice turned something readable into a raw key, on the primary action of the
    screen. The TS union and the status filter carried the same phantom value, so both now mirror
    TopupRequestStatus in Modules.Billing.Contracts.
  • Two safety nets, because catalog parity structurally cannot see run-time keys (both locales can
    be missing the same key and still match): parseMissingKeyHandler degrades a missing key to its
    last segment and warns in development, and tests/i18n/status-keys.spec.ts reads the members
    straight out of the backend enums and asserts each resolves in both catalogs. Mutation-checked.
  • tests/i18n/i18n.spec.ts read the header off a variable its own route handler assigns, but
    waitForRequest resolves at dispatch, before the handler runs. It reads the resolved request now.
  • The deprecated NAV_ITEMS export is gone. Dead since the nav moved to sections/topNavTop
    (no importer in src/ or tests/), and it carried hardcoded English labels that would have
    shipped untranslated the moment anyone imported it.

Infra carve-outs, corrected after review. Two things in the out-of-topic hunks were wrong and
are fixed on the branch:

  • The MinIO carve-out only moved minio/minio to quay.io. minio/mc is gone from Docker Hub too
    (hub.docker.com/v2/repositories/minio/mc/ answers 404) and it is what minio-init runs, so both
    dotnet run --project src/Host/FSH.Starter.AppHost and docker compose up died on the pull and the
    fsh bucket was never created. Now pinned to the same quay tag #1388 uses.
  • The SSH.NET pin is gone: it pinned nothing. Its own comment claimed bumping Testcontainers
    does not help, but 4.14.0 — which this branch also carries — declares SSH.NET >= 2026.0.0.
    Measured rather than argued: with the pin removed, dotnet restore src/FSH.Starter.slnx --force
    reports zero NU1902/NU1903 and exits 0. (The MessagePack pin next to it stays; removing that one
    does bring its advisory straight back.)

With both applied, deploy/docker/docker-compose.yml and src/Directory.Packages.props are now
genuinely byte-identical to #1388 (git diff --exit-code, checked today), which the earlier claim
was not.


Second review round.

The role detail screen rendered eight group headings, eight blurbs and thirty-four permission
rows straight out of PERMISSION_CATALOG, all English literals: a pt-BR operator opening a role
read a translated shell wrapped around fifty English strings, the largest untranslated surface
left in the app. Each group carries a stable key now, the English text stays in the file as the
fallback, and the screen resolves through the roles catalog.
tests/i18n/status-keys.spec.ts gates it by resolving the permission constants the way the app
does at run time (the catalog holds the permission value, Permissions.Users.Create, not the
identifier it is written with) and asserting every group, blurb and entry exists in both locales.

Seven call sites formatted dates with toLocaleString() and friends, which follow the browser,
not the app: a browser in en-US showed 5/23/2026, 10:00:00 AM beside Portuguese labels.
format.ts gains formatDateTime/formatTime and the call sites go through resolveLocale.
The impersonation card's "started … · expires …" was hardcoded English prose; it is a key now.

Three tests that could not fail, all named in review: format.spec.ts passed an explicit locale
in all nine assertions, so resolveLocale (the branch every production call takes) was never
exercised; html[lang] had no assertion at all while the PR claimed screen readers now see the
real language; and i18n.spec.ts read the Accept-Language header off a variable its own route
handler assigns, but waitForRequest resolves at dispatch, before the handler runs. It reads the
resolved request now.

And one real defect in the fallback this PR introduced. parseMissingKeyHandler took only
the key and returned its capitalized last segment. i18next calls that handler for a missing key
whether or not the call site passed a defaultValue, and the handler's return value is what
renders, so every t(key, { defaultValue }) in the app was being degraded to a truncation of its
own key. Measured against the installed i18next (26.3.6) before the fix:
t("perm.entry.Permissions.Users.Create", { defaultValue: "Create users" }) returned "Create".
The two rules (a caller's fallback wins, otherwise degrade to the last segment) moved to
lib/i18n-fallback.ts, and tests/i18n/missing-key.spec.ts drives them through a real i18next
instance rather than re-implementing the contract. Reverting the guard turns it red.

Third round: numbers, plurals and the upload errors.

Counts went into the string raw. {{count}} and the named count placeholders interpolate the
number with no formatter, so a Portuguese UI read "1234" beside currency and dates on the same
screen that were correctly grouped. Every count (it is the plural selector, so always numeric)
and the named ones, checked one at a time, go through i18next's number formatter now. Two are
deliberately left alone: the files dropzone interpolates already formatted byte sizes, and the
activity page pre-formats its own count.

EntityPageHeader rendered its count chip as `${unit}s`, an English pluralization rule
applied to every language, and four pages passed the unit as an English literal. "organização" +
"s" is not a word. It takes the unit.* plural keys now, the way clients/dashboard's header
already did, with the token typed as a union so an already translated word cannot be handed to it
and render as a missing key.

Upload failures reached the user in English: cancel, transport failure, a rejected PUT, a blocked
extension, an oversize file. They were prose built inside the hook and in module-scope XHR
handlers. They raise an UploadError carrying a catalog key now, and one exported resolver turns
it into text wherever it is shown.

The new format.spec.ts case asserts both halves of the chip on a real list under pt-BR
(1.234 organizações): reverting the formatter or the plural keys turns it red.

One content change rode along with the plural keys and was not called out at the time: the
notifications inbox chip counted "N items" and now counts "N notificações" / "N notifications".
The page was passing unit="item" to a header that appended an "s"; moving to the typed unit
union meant naming what is actually being counted, and unit="notification" is that. The unit is
a deliberate wording change, not a side effect of the formatter.

Fourth round: the three that were declared rather than fixed.

The language detector kept i18next's default i18nextLng storage key. Every other value this app
persists is namespaced (fsh.admin.accessToken, fsh.admin.theme,
fsh.admin.sidebar.collapsed), and the bare key is claimed by both apps on a shared origin and by
any other i18next app deployed beside them. It is fsh.admin.lng now. The migration cost is one
session: a returning user's old value is not read, so the first paint after deploy falls to the
browser locale or the deployment default, and the profile hydrate then restores User.Locale.

format.ts reactivity turned out to hold, and now has a gate that says why rather than an
assurance that it probably does. resolveLocale reads i18n.language when the formatter runs,
so a date formatted on a previous render keeps its locale until something re-renders the
component; nothing in format.ts subscribes to languageChanged. What makes the switch reach a
mounted list is that every component in this app that formats also calls useTranslation (checked
across all of src), and that subscription is easy to drop in a refactor with no test noticing.
The new format.spec.ts case renders an invoice date under en-US, drives the real switcher to
pt-BR, and asserts the same list now reads 01 de mai. de 2026 with the list read exactly once.
Freezing resolveLocale to the language captured at module load turns it red.

Upload-error localization now has a gate of its own: tests/i18n/upload-errors.spec.ts drives the
real avatar picker on /settings/profile under ?culture=pt-BR through a storage PUT that never
connects, one the bucket rejects with a 403, and a presign that never leaves the browser. The 403
case asserts the interpolated {{status}}, not just that some catalog string rendered. Writing it
found one more English string on that path: describeUploadError returned e.message for any
plain Error, and apiFetch does not wrap fetch, so a presign that never reached the API
surfaced as the browser's own TypeError("Failed to fetch") ahead of the localized fallback the
caller had already passed in. That branch logs the original for diagnosis and returns the catalog
string now; restoring return e.message turns the third case red.

The one item left deliberately as-is is the Playwright assertion budget. expect.timeout is
10 s rather than the 5 s default, which aligns it with the action (10 s) and navigation (15 s)
budgets already in this config: every test ends in a toBeVisible, and under CPU contention the
first paint of a lazy route lands past 5 s while staying well inside the other two. It is a wait
budget, not a correctness threshold, and it is flagged here so it can be vetoed rather than
discovered.

Admin slice of the i18n work (split of fullstackhero#1344 as requested in review).
Self-contained: it needs nothing from the backend slices, and the backend needs
nothing from it.

- `react-i18next` wiring in `src/i18n.ts`, language detected from the stored
  preference and negotiated with the API through `Accept-Language`.
- English and Brazilian Portuguese catalogs, split per feature namespace.
- Language switcher in the topbar; the chosen language is persisted to the user
  profile so it survives a reload, and `html[lang]` follows it through a
  `languageChanged` listener rather than staying pinned to `en`.
- Number, date and currency formatting moved to `src/lib/format.ts` so the
  presentation layer owns formatting. The API stays UI-culture-only.
- Impersonation handoff carries `locale` in the URL so an operator keeps their
  language when landing in the tenant app. Harmless on its own: nothing reads
  the parameter until the dashboard slice ships.
- Playwright specs pin catalog parity (keys and placeholders, both directions),
  the switcher, formatting and the handoff parameter.

The `SSH.NET` pin (`2026.0.0`) rides along because `template-smoke.yml` runs on
`clients/**` and builds the scaffolded solution, which fails `restore` with
`NU1903` until fullstackhero#1333 merges. It is byte-identical to that PR.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a10d397626

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread clients/admin/src/api/users.ts
Comment thread clients/admin/src/env.ts
… config

env.ts reads defaultLanguage from config.json, but neither supported deployment
path emitted it: the Docker template and the Terraform runtime_config carried
only apiBase, defaultTenant and dashboardUrl. Every production deployment
therefore fell back to en-US and the advertised per-deployment default language
was configurable only in the Vite development file.

The entrypoint defaults the variable to en-US so an unset value and an absent
config.json land on the same language, and initI18n already drops an
unsupported tag back to en-US.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

…advisories

`dotnet restore` fails for the whole solution under `TreatWarningsAsErrors`, on
`main` and on every open PR alike. Advisory-database drift, not a regression from
any change: a commit green on 2026-08-10 is red today with no edits.

- `Testcontainers.PostgreSql` / `.Redis` / `.Minio` 4.11.0 -> 4.14.0 (NU1903,
  GHSA-q939-rpr3-3284). 4.11.0 depends on `SSH.NET` 2025.1.0; 4.14.0 already
  depends on the patched 2026.0.0, so the advisory clears with no transitive pin
  to remember to remove later. Same fix as fullstackhero#1369, so the two do not conflict.
- `Microsoft.SourceLink.GitHub` 8.0.0 -> 10.0.401 (NU1902,
  GHSA-23fw-v26w-5fgq). 8.0.0 drags in `Microsoft.Build.Tasks.Git` 8.0.0 and the
  8.x line has no patched release, so a transitive pin cannot fix it; the package
  itself has to move. 10.0.401 depends on `Microsoft.Build.Tasks.Git` 10.0.401,
  past the patched 10.0.303. Build-time only (`PrivateAssets="all"`), referenced
  only where `IsPackable == true`, which is the CLI alone - and `src/Tools/**` is
  excluded from the template, so the scaffold never sees it.

Verified: `dotnet restore src/FSH.Starter.slnx` exits 0 with no NU19xx, and
`dotnet build src/FSH.Starter.slnx -c Release -warnaserror` reports 0 warnings
and 0 errors.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

MinIO withdrew `minio/minio` from Docker Hub. Docker Hub's API now answers
`object not found` for the repository, and a pull fails with:

    pull access denied for minio/minio, repository does not exist or may
    require 'docker login'

That takes down every Testcontainers-backed integration test (the harness boots
a MinIO container per fixture, so all 724 tests in `Integration.Tests` fail at
container start), the Aspire AppHost, and the Docker Compose deployment. The
image is still published at `quay.io/minio/minio`:

- `Integration.Tests` and `Integration.Middleware.Tests` harnesses
- `AppHost.cs`, via Aspire's `WithImageRegistry` / `WithImageTag`
- `deploy/docker/docker-compose.yml` and the image table in its README

The tag is pinned to `RELEASE.2025-09-07T16-13-09Z` rather than `:latest`. quay
has not moved `:latest` since 2025-09-07, so the two resolve to the same digest
today; pinning only removes the surprise of a silent move later, and keeps the
test harness off a floating tag. Whether to track a newer release, or a different
S3-compatible image, is a separate call.

While in the README's image table: `postgres` and `redis` rows had drifted from
what compose actually ships (`postgres:18-alpine`, `valkey/valkey:9.1.0-alpine`).

Verified: `docker pull minio/minio:latest` fails with the error above;
`docker pull quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z` succeeds
(`sha256:14cea493d9a34af32f524e538b8346cf79f3321eff8e708c1e2960462bd8936e`, the
same digest `:latest` resolves to). `dotnet test Integration.Tests -c Release`
passes against the pinned image, and the Aspire manifest renders the container
as `quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z`.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

The server rotates the refresh token on every successful refresh, so two
refreshes started close together send the same token twice: the loser
gets a 401 and apiFetch's failure path calls tokenStore.clear(). The
operator is signed out mid-work, with no message, because the language
switcher swallows the error with .catch(() => undefined).

The single-flight existed but lived inside apiFetch's 401 retry, so it
only covered one of the three call sites. It moves into
refreshAccessToken itself, which is the only place that knows a refresh
is in flight; the other two callers (session bootstrap, the language
switcher) now share it for free.

Gate: the new spec switches language twice in a row against a refresh
that is held open, and counts the calls. Two on the previous code, one
after. tsc and lint clean, full admin Playwright suite 136 passed.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@marcelo-maciel

Copy link
Copy Markdown
Contributor Author

Follow-up left out of this PR on purpose:

The language-switcher mutation has no onError. In clients/admin/src/components/layout/topbar.tsx, a failed updateMyProfile leaves the UI in the newly selected language with nothing persisted and no feedback to the operator. The next reload silently reverts it. A toast plus reverting i18n.changeLanguage would close it; it is one mutation callback and does not belong in the single-flight fix this PR carries.

The single-flight added earlier stopped two *concurrent* refreshes from racing,
but one failed refresh was still enough on its own: `refreshAccessToken()`
cleared the token store on any non-ok response, and the language switcher fires
it speculatively (it only re-mints the JWT so the new `locale` claim is issued).
A refresh token that had been revoked, rotated in another tab or dropped by a
reseed therefore signed the operator out for choosing a language.

Ending the session now belongs to the callers that know the request needed auth:
the 401 retry in `apiFetch` and the boot probe in `AuthProvider`, which already
cleared. The switcher reports the failure instead of swallowing it.

A failed save is reported too. The language mutation had an `onSuccess` and no
`onError`, so a rejected `PUT /identity/profile` left the UI switched with
nothing on screen to say the choice was not stored — it silently reverts on the
next fresh mount, which reads as the app forgetting on its own.

Both paths are covered: a 401 refresh keeps the session and the language, and a
500 save surfaces the toast.
Matching key sets do not catch a translation that drops or renames {{var}}:
i18next renders the placeholder as literal text, or the value is silently lost,
and no key is missing. The gate compares the variable set per key across
locales, ignoring the formatter after the comma.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Approving a top-up moves it to `Invoiced`, and the badge read "status.invoiced":
the label key is built from the value the API sends, and the catalog had neither
`status.invoiced` nor `status.cancelled`. It did have `status.approved`, which
the backend never emits. On main the badge printed the raw enum name, so this
slice turned something readable into a key.

The TS union and the filter list carried the same phantom, so both now mirror
`TopupRequestStatus` in Modules.Billing.Contracts (Pending, Invoiced, Completed,
Rejected, Cancelled) — filtering by "Approved" could only ever return nothing.

Two safety nets, because catalog parity cannot see keys that are built at run
time (both locales can be missing the same one and still match):

- `parseMissingKeyHandler` degrades a missing key to its last segment and warns
  in development, so the worst case is the un-localized name rather than the key.
- `tests/i18n/status-keys.spec.ts` reads the members straight out of the backend
  enums and asserts each one resolves in both catalogs. Verified by mutation:
  removing `status.invoiced` from the pt-BR catalog turns it red.

Also removes the deprecated `NAV_ITEMS` / `filterNavItems` export, dead since the
nav moved to `sections`/`topNavTop` (no importer in src/ or tests/): it carried
hardcoded English labels that would have shipped untranslated the moment anyone
imported it.

`tests/i18n/i18n.spec.ts` read the Accept-Language header off a variable its own
route handler assigns, but `waitForRequest` resolves at dispatch, before the
handler runs. It reads the resolved request instead.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

The MinIO carve-out this branch carries only moved `minio/minio`. `minio/mc` is
gone from Docker Hub as well (`hub.docker.com/v2/repositories/minio/mc/` answers
404), and it is what `minio-init` runs: without it `dotnet run --project
src/Host/FSH.Starter.AppHost` and `docker compose up` both die on the image pull,
and the `fsh` bucket is never created, so the first upload fails with
NoSuchBucket.

Same pinned tag as fullstackhero#1388, which owns the fix, so the copy stays byte-identical
to it and can be dropped once that lands.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

The pin's own comment says "Testcontainers 4.11.0 and 4.13.0 both depend on
2025.1.0, so bumping Testcontainers does not help", but the branch also bumps
Testcontainers to 4.14.0, whose nuspec declares `SSH.NET >= 2026.0.0`. The two
statements cannot both be true, and the bump is the one that is: with the pin
removed, `dotnet restore src/FSH.Starter.slnx --force` reports zero NU1902/NU1903
and exits 0. It was carrying a transitive pin that no longer pins anything.

The MessagePack pin above it stays: that one is still load-bearing (removing it
brings GHSA-hv8m-jj95-wg3x straight back, verified in the same probe).
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

… the browser

The role detail screen renders eight group headings, eight blurbs and thirty-four
permission rows straight out of `PERMISSION_CATALOG`, all of them English
literals. A pt-BR operator opening a role read a fully translated shell wrapped
around fifty English strings — the largest untranslated surface left in the app.
Each group now carries a stable key, the English text stays in the file as the
fallback, and the screen resolves through the `roles` catalog.

`tests/i18n/status-keys.spec.ts` gates it: it resolves the permission constants
the same way the app does (the catalog holds the permission *value* at run time,
not the identifier it is written with) and asserts every group, blurb and entry
has an entry in both locales.

Seven call sites formatted dates with `toLocaleString()` and friends, which use
the browser's locale, not the app's — so a browser in en-US showed
`5/23/2026, 10:00:00 AM` next to Portuguese labels. `format.ts` gains
`formatDateTime`/`formatTime` and the call sites use them. The impersonation
card's "started … · expires …" was hardcoded English prose; it is a key now.

Three tests that could not fail, all of them named in review:

- `format.spec.ts` passed an explicit locale in all nine assertions, so
  `resolveLocale` — the branch every production call takes — was untested.
- `html[lang]` had no assertion at all, while the PR claims screen readers and
  browser translation now see the real language.
- `i18n.spec.ts` read the `Accept-Language` header off a variable its own route
  handler assigns, and `waitForRequest` resolves at dispatch, before the handler
  runs. It reads the resolved request now.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

`parseMissingKeyHandler` took only the key and returned the capitalized last
segment. i18next calls it for a missing key whether or not the call site passed
a `defaultValue`, and the handler's return value is what renders — so every
`t(key, { defaultValue })` in the app was silently degraded to a truncation of
its own key. The permission matrix was the visible case: an entry the catalog
had not caught up with rendered "Create" where the fallback says "Create users".

Confirmed against the installed i18next (26.3.6) before the fix:
`t("perm.entry.Permissions.Users.Create", { defaultValue: "Create users" })`
returned `"Create"`, and the handler's second argument arrived as the
defaultValue (`null` when there is none, not `undefined`).

The two rules — a caller's fallback wins, otherwise degrade to the last segment
— move to `lib/i18n-fallback.ts`, and `tests/i18n/missing-key.spec.ts` drives
them through a real i18next instance rather than re-implementing the contract.
Reverting the guard turns the first of the three red.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

…ound

Three things a PR claiming i18n coverage should not have left.

**Counts were interpolated raw.** `{{count}}` and the page/total placeholders put
the number in with no formatter, so a Portuguese UI read "1234" next to currency
and dates that were correctly grouped. i18next's own `number` formatter runs Intl
with the active language, so the fix is per catalog entry: every `count` (it is
the plural selector, so it is always numeric) plus the named ones checked one at a
time. Two are deliberately left alone — the files dropzone interpolates already
formatted byte sizes, and the activity page pre-formats its own count.

**The list header pluralized in English.** `EntityPageHeader` rendered
`${unit}s`, and four pages passed the unit as an English literal. "organização" +
"s" is not a word. It takes the `unit.*` plural keys now, the way the dashboard's
header already does, with the token typed as a union so an already translated word
cannot be handed to it and silently render as a missing key.

**Upload failures reached the user in English.** Cancel, transport failure, a
rejected PUT, a blocked extension and an oversize file were built as English
prose inside the hook and in module-scope XHR handlers. They now raise an
`UploadError` carrying a catalog key (namespaced, since the resolver runs with
whatever `t` the display site is bound to), and one exported resolver turns it
into text at every place that shows it.

The new `format.spec.ts` case asserts both halves of the count chip on a real
list under pt-BR: reverting either the formatter or the plural keys turns it red.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

`defaultValue: e.messageKey` would have rendered "common:upload.cancelled" on
screen if the catalog ever lost the entry. Without it the missing-key handler
degrades to "Cancelled", which is the readable floor it exists to provide.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

… gate both

Three loose ends the review left open on this branch.

**Storage key.** The detector kept i18next's default `i18nextLng`. Every other
value this app persists is namespaced (`fsh.admin.accessToken`,
`fsh.admin.theme`, `fsh.admin.sidebar.collapsed`), and the bare key is claimed
by both apps on a shared origin and by any other i18next app deployed beside
them. Now `fsh.admin.lng`. Migration cost is one session: a returning user's
old value is not read, so the first paint after deploy falls to the browser
locale or the deployment default, and the profile hydrate then restores
`User.Locale`. The prose that named the old key follows it.

**Upload failures.** `describeUploadError` returned `e.message` for any plain
`Error`, and `apiFetch` does not wrap `fetch`, so a presign step that never
reaches the API surfaced as the browser's own `TypeError("Failed to fetch")` -
in English, under a Portuguese UI, ahead of the localized fallback the caller
had already passed in. That branch now logs the original for diagnosis and
returns the catalog string.

**Gates.** `tests/i18n/upload-errors.spec.ts` drives the real avatar picker on
/settings/profile under `?culture=pt-BR` through three failures: a storage PUT
that never connects, one the bucket rejects with a 403 (the interpolated
`{{status}}` is asserted, not just the key), and a presign that never leaves
the browser. `format.spec.ts` gains the reactivity case: a date already on
screen has to reformat when the switcher changes the language under it.
`resolveLocale` reads `i18n.language` at call time and nothing in format.ts
subscribes to `languageChanged`; what makes it work is that every component
that formats also calls `useTranslation`, which a refactor can drop silently.

Verified: `playwright test tests/i18n/upload-errors.spec.ts` 3/3 and
`tests/i18n/format.spec.ts` 4/4. Mutations: returning `e.message` from the
fallback branch fails the presign case; freezing `resolveLocale` to the
language captured at module load fails the reactivity case. `tsc -b` and
`eslint .` both exit 0.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant